Recognize a VB exception filter's type test - #4134
siegfriedpammer merged 3 commits into
Conversation
|
Did you use AI to implement this? Please read https://github.com/icsharpcode/ILSpy/blob/master/CONTRIBUTING.md#contributing especially bullet 2. |
| @@ -0,0 +1,109 @@ | |||
| .assembly extern System.Runtime | |||
There was a problem hiding this comment.
Why not make this a VBPretty test?
There was a problem hiding this comment.
Yeah, my bad. Will change that in a sec
There was a problem hiding this comment.
should be done, sorry, won't happen again
There was a problem hiding this comment.
no worries, it's just that IL pretty tests are a last resort kind of thing when something cannot be expressed as C#/VB test and LLMs somehow prefer to write IL pretty tests.
|
Hi. I didn't use the AI to write the code itself. I did however use AI to make it easier to read. Same with the comment - i wrote it myself and then told ai to please make it look nicer |
| Return 0 | ||
| End Function | ||
|
|
||
| Friend Sub VBFunction(value As Object) |
There was a problem hiding this comment.
this seems to be a rather special case, namely On Error GoTo which we don't really have plans on supporting.
| if (condition is not Comp comp || !comp.Right.MatchLdNull()) | ||
| return false; | ||
| // `cgt.un x, null` is how both compilers spell `x != null` for a reference. | ||
| if (comp.Kind != ComparisonKind.Inequality |
There was a problem hiding this comment.
not sure why your clanker thinks that handling ComparisonKind.GreaterThan is necessary here...
|
I will add some more tests to the PR and simplify the code a bit, if you don't mind. |
vbc builds an exception filter as a single non-short-circuiting
expression: `isinst`, then the user's `when` conditions, combined with
`and`. DetectCatchWhenConditionBlocks only knew the branch chain csc
emits, so the type test stayed inside the filter and the handler kept the
`object` variable it has in IL:
catch (object obj) when ((obj is Exception) & (num2 != 0) & (num == 0))
{
ProjectData.SetProjectError((Exception)obj);
A catch type has to derive from Exception, so that does not compile.
The conjunction form is matched too now, lifting the test to the catch
type as the block form already does. The other conjuncts stop running for
a non-matching exception once the test moves, so the filter has to be
pure for this to be invisible; PropagateExceptionVariable then drops the
castclass in the handler:
catch (Exception ex) when ((num2 != 0) & (num == 0))
{
ProjectData.SetProjectError(ex);
Closes icsharpcode#3659
Only vbc's On Error lowering produces this filter; structured Catch...When filters call SetProjectError and never reach it. Roslyn emits it from a single code path with System.Exception as the type, and legacy vbc output has the same shape, so there is no chain of unknown conjuncts to walk. The transform runs before the expression transforms canonicalize comparisons, so the filter still compares with cgt against ldnull and cgt.un against zero: the ldnull comparison is fixed up first, and MatchCompUnsignedZero accepts both the unsigned and canonical form. Assisted-by: Claude:claude-opus-5:Claude Code
87d2246 to
4a3abcf
Compare
|
Thank you for your contribution! |
VB lowers error handling differently from C# and no fixture covered it: every Catch brackets its body with SetProjectError/ClearProjectError, Catch...When moves that call into the filter, and On Error becomes one try block with a dispatch switch driven by line and handler state. The expected output is the current decompilation, rough spots included (delegate-invoked filter blocks; gotos into the dispatch switch that leave unreachable code, which keeps VBOnError.cs out of the warnings-as-errors project), so improvements show up as diffs. IL offset labels differ per compiler and optimization level, hence the per-method #if variants. Legacy vbc output was verified on Windows. The correctness test covers what a pretty test cannot: catching object made the decompiled On Error code fail to recompile with CS0155, and the round trip shows the rewritten dispatch code still runs the same. Like the mcs configurations, the legacy and Roslyn 2.10/.NET Core 2.2 ones recompile the decompiled VB code with the latest Roslyn: C# 5 cannot express the exception filters VB error handling compiles to, and the .NET Core 2.2 Microsoft.VisualBasic.dll lacks Information.Err and ProjectData.CreateProjectError. Assisted-by: Claude:claude-opus-5:Claude Code
4a3abcf to
483a866
Compare
vbc emits a filter as one
andexpression rather than the branch chain csc emits, so VB filters decompiled tocatch (object obj).That form is now matched and becomes
catch (Exception ex) when (...), but only when the whole filter is pure, since moving the type test changes evaluation order.Closes #3659